Skip to content

Remove the duplicated comment #146

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jun 27, 2025
Merged

Remove the duplicated comment #146

merged 1 commit into from
Jun 27, 2025

Conversation

@bjk7119
Copy link
Contributor

@bjk7119 bjk7119 commented Jun 12, 2025 •
edited
Loading

Description

  • Check if OssItem Instance for an OSS is already present
    • Use same OssItem instance if the oss name and version are same
  • Replace '+=' to '=' when append comment

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Documentation update
  • Refactoring, Maintenance
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

@bjk7119 bjk7119 requested review from dd-jy and soimkim June 12, 2025 07:04
@bjk7119 bjk7119 self-assigned this Jun 12, 2025
@bjk7119 bjk7119 added the chore [PR/Issue] Refactoring, maintenance the code label Jun 12, 2025
dd-jy
dd-jy reviewed Jun 12, 2025
View reviewed changes
src/fosslight_binary/_binary.py Outdated

if nvd_url and len(nvd_url) > MAX_EXCEL_URL_LENGTH:
oss.comment += f"\nExceeded the maximum vulnerability URL length of {MAX_EXCEL_URL_LENGTH} characters."
oss.comment = f"\nExceeded the maximum vulnerability URL length of {MAX_EXCEL_URL_LENGTH} characters."
Copy link
Contributor

@dd-jy dd-jy Jun 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bjk7119
다른 로직에서 comment 추가될 수 있다면, +=로 두는 것이 맞지 않는지 의견드립니다.

Copy link
Contributor Author

@bjk7119 bjk7119 Jun 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oss.comment가 FL Util > OssItem 클래스의 setter로 쓰이면서
FL Util의 PR(https://github.com/fosslight/fosslight_util/pull/219/files)에서 new_comment로 쓰여 굳이 +가 없어도 될 것 같아 삭제하였습니다.

@bjk7119 bjk7119 force-pushed the exclude branch 2 times, most recently from 560e97f to 1666c8b Compare June 23, 2025 06:01
@bjk7119 bjk7119 changed the title Minor modification to append comment Remove the duplicated comment Jun 23, 2025
@bjk7119 bjk7119 merged commit 1095281 into main Jun 27, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dd-jy dd-jy dd-jy approved these changes

@soimkim soimkim Awaiting requested review from soimkim

Assignees

@bjk7119 bjk7119

Labels

chore [PR/Issue] Refactoring, maintenance the code

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bjk7119 @dd-jy